Codecov Report

Merging #1497 (1d508ba) into main (beba414) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1497      +/-   ##
==========================================
- Coverage   84.72%   84.71%   -0.01%     
==========================================
  Files         156      155       -1     
  Lines        4784     4778       -6     
==========================================
- Hits         4053     4047       -6     
  Misses        731      731              

Do not merge yet.

Pushing temporary debug code to investigate windows build failure.

diff --git a/examples/http/server.cc b/examples/http/server.cc
index cf7e6bb..4687d1c 100644
--- a/examples/http/server.cc
+++ b/examples/http/server.cc
@@ -3,6 +3,13 @@

#include "server.h"
#include "opentelemetry/trace/context.h"
+
+// DEBUG: investigating build break
+
+#ifdef DELETE
+#error "Someone broke semantic_conventions"
+#endif
+
#include "opentelemetry/trace/semantic_conventions.h"
#include "tracer_common.h"

LGTM. Thanks for picking this, it was long pending item :)

It would be good to document this script execution somewhere. One of the options could be in RELEASING.md if we want to recommend generating semantic conventions before every release. Or anywhere is fine as long as it is documented.

Thanks @lalitb

Added documentation.

There is a very weird build break on windows,
I suspect some define pollution.

Trying to investigate that with CI, and might need some help (I don't have a windows os).

Do not merge yet.

Resolved and ok to merge.

On windows, some header file causes to define the symbol DELETE,
which conflicts with semantic convention FaasDocumentOperationValues::DELETE.

Implemented a work around, and CI on windows now passes.

Revisited the windows work around for DELETE.

Now that the root cause is well identified, and affects one platform only,
implementing the work around -- for that platform -- in the template file is (more) acceptable:

• this is the best solution, for ease of use. Many users are likely to experience build breaks without the work around because winnt.h is hard to avoid for people using this platform, so we might as well provide it for everyone using windows in the generated file semantic_conventions.h
• the examples are now clean of any work around

Also, the generate script now discloses which semantic convention is compiled, with:

• -Dsemconv=trace
• -Dsemconv=resource

This will make implementing work around for other issues, should that be necessary, easier.

@ThomsonTan Thanks for finding the offending header.

@lalitb Last item about the DELETE work around now resolved (in a better way, in the template)

Ready to merge.

Thanks @marcalff , @ThomsonTan . Merging it.